feat!: added final job statuses middleware handler on crud operations (MAPCO-9859)#56
Conversation
CL-SHLOMIKONCHA
changed the title
CL-SHLOMIKONCHA
changed the title
| const jobManager = container.resolve(JobManager); | ||
| const logger = container.resolve<Logger>(SERVICES.LOGGER); | ||
|
|
||
| const finalStateStatuses: OperationStatus[] = [OperationStatus.COMPLETED, OperationStatus.ABORTED, OperationStatus.EXPIRED]; |
There was a problem hiding this comment.
Worth to define it in more upper level, even more FINAL states def might be reused in other mapcolonies domain(APP for instance)
There was a problem hiding this comment.
As we discussed, the change should be included in reusing this middle ware below comment on abort logic, finalStatuses will remain in this level as it the middle ware logic that should define it
There was a problem hiding this comment.
i reused it below comment
| router.get('/parameters', jobsController.getJobByJobsParameters); | ||
| router.get('/:jobId', jobsController.getResource); | ||
| router.put('/:jobId', jobsController.updateResource); | ||
| router.put('/:jobId', validateJobStatusMiddleware, jobsController.updateResource); |
There was a problem hiding this comment.
It's not exactly middleware pattern midlleware can come with routes.
Usage should like app.use(validateJobStatusMiddleware(params1, ...));
There was a problem hiding this comment.
Not accurate, middlewares are also can be defined Route-Level as i did it here, not only Global / Application-Level if you want to it run on specific routes
| throw new NotFoundError(message); | ||
| } | ||
| if ((jobEntity.status as OperationStatus) === OperationStatus.COMPLETED || (jobEntity.status as OperationStatus) === OperationStatus.ABORTED) { | ||
| if (jobEntity.status === OperationStatus.COMPLETED || jobEntity.status === OperationStatus.ABORTED) { |
There was a problem hiding this comment.
here can be used [OperationStatus.COMPLETED, OperationStatus.ABORTED, OperationStatus.EXPIRED] defined previously
There was a problem hiding this comment.
Why didn't you put expired here?
How come it wasn't found in the tests?
There was a problem hiding this comment.
fixed, reuse the middle ware instead to check job status it inside the function
| throw new NotFoundError(message); | ||
| } | ||
| if ((jobEntity.status as OperationStatus) === OperationStatus.COMPLETED || (jobEntity.status as OperationStatus) === OperationStatus.ABORTED) { | ||
| if (jobEntity.status === OperationStatus.COMPLETED || jobEntity.status === OperationStatus.ABORTED) { |
There was a problem hiding this comment.
Why didn't you put expired here?
How come it wasn't found in the tests?
4 participants
Added task creation condition for an aborted job